Skip to content

Reject empty keypaths separator#580

Merged
fabiocaccamo merged 3 commits into
fabiocaccamo:mainfrom
CodingFeng101:codex/small-fix
Jul 7, 2026
Merged

Reject empty keypaths separator#580
fabiocaccamo merged 3 commits into
fabiocaccamo:mainfrom
CodingFeng101:codex/small-fix

Conversation

@CodingFeng101

Copy link
Copy Markdown
Contributor

What changed

keypaths(..., separator="") now raises the same ValueError as other invalid separators instead of silently falling back to the default "." separator.

Why

The function documents and reports that the separator must be a non-empty string. An explicit empty separator is most likely a caller mistake, and silently converting it to "." can hide the invalid input and return keypaths with an unexpected separator.

separator=None still preserves the previous default behavior.

Validation

  • python -m pytest tests\core\test_keypaths.py -q
  • python -m pytest tests\core -q
  • python -m ruff check benedict\core\keypaths.py tests\core\test_keypaths.py
  • python -m black --check benedict\core\keypaths.py tests\core\test_keypaths.py
  • git diff --check

@codecov

codecov Bot commented Jul 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.33%. Comparing base (c7c934f) to head (7c5f958).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #580      +/-   ##
==========================================
- Coverage   98.33%   98.33%   -0.01%     
==========================================
  Files          64       64              
  Lines        2400     2399       -1     
==========================================
- Hits         2360     2359       -1     
  Misses         40       40              
Flag Coverage Δ
unittests 98.33% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens validation in benedict.core.keypaths.keypaths() by treating an explicit empty separator="" as invalid input (raising ValueError) rather than silently defaulting to ".", aligning runtime behavior with the documented contract.

Changes:

  • Change separator defaulting logic to only fall back to "." when separator is None (not when it is an empty string).
  • Add validation to reject empty-string separators with a ValueError.
  • Add a unit test asserting separator="" raises ValueError.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
benedict/core/keypaths.py Makes separator defaulting explicit (None only) and raises on empty separators.
tests/core/test_keypaths.py Adds coverage for rejecting an empty separator.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread benedict/core/keypaths.py
Comment thread tests/core/test_keypaths.py
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
fabiocaccamo
fabiocaccamo previously approved these changes Jul 7, 2026
@fabiocaccamo fabiocaccamo merged commit c987439 into fabiocaccamo:main Jul 7, 2026
18 checks passed
fabiocaccamo added a commit that referenced this pull request Jul 7, 2026
Co-Authored-By: Fabio Caccamo <fabio.caccamo@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants